Add RoomNav support #182
Add RoomNav support #182
Conversation
facebook-github-bot
added
the
CLA Signed
|
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status. |
There was a problem hiding this comment.
@JasonJiazhiZhang and @medhini that's amazing seeing new task is coming with PPO baselines. Left some comments. I think it's in pretty mature stage, but need some polishing.
| def __init__(self, sim: Simulator, config: Config): | ||
| self._sim = sim | ||
| self.room_name_to_id = { | ||
| "bathroom": 0, |
| @@ -377,6 +445,81 @@ def update_metric(self, episode, action): | |||
| ) | |||
|
|
|||
|
|
|||
| @registry.register_measure | |||
| class RoomNavMetric(Measure): | |||
| r"""RoomNavMetric - SPL but for RoomNav | |||
There was a problem hiding this comment.
Name it more specific: RoomNavSPL maybe.
| def get_reward(self, observations): | ||
| reward = self._rl_config.SLACK_REWARD | ||
|
|
||
| current_target_distance = self._distance_target() |
There was a problem hiding this comment.
current_target_distance isn't used.
| self._rl_config = config.RL | ||
| self._core_env_config = config.TASK_CONFIG | ||
|
|
||
| self._previous_target_distance = None |
There was a problem hiding this comment.
current_target_distance isn't used.
|
|
||
| self._previous_target_distance = None | ||
| self._previous_action = None | ||
| self._episode_distance_covered = None |
There was a problem hiding this comment.
same _episode_distance_covered
|
|
||
|
|
||
| def run_exp(exp_config: str, run_type: str, opts=None) -> None: | ||
| r"""Runs experiment given mode and config |
There was a problem hiding this comment.
Usually main method is the last one in the file:
| r"""Runs experiment given mode and config | |
| r"""Runs an experiment with given mode and config. |
|
|
||
| room_id: str = attr.ib(default=None, validator=not_none_validator) | ||
| room_name: Optional[str] = None | ||
| room_aabb: Tuple[float] = attr.ib( |
There was a problem hiding this comment.
Let's move all RoomNav Task related definitions to habitat/tasks/nav/room_nav_task.py and include it here. This Nav task file already grew too much and is hard to read.
| room_aabb: Tuple[float] = attr.ib( | ||
| default=None, validator=not_none_validator | ||
| ) | ||
| # room_id: str = attr.ib(default=None, validator=not_none_validator) |
There was a problem hiding this comment.
Remove commented out code.
| @pytest.mark.skipif( | ||
| not baseline_installed, reason="baseline sub-module not installed" | ||
| ) | ||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
This is amazing way to cover all trainers smoke test with one parameterized test. Thank you for implementing it.
|
|
||
|
|
||
| @registry.register_dataset(name="RoomNav-v1") | ||
| class RoomNavDatasetV1(Dataset): |
There was a problem hiding this comment.
Maybe we can reduce code duplication if inherit from PointGoalDatasetV1 or create common parent class.
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Motivation and Context
This PR adds RoomNav support to habitat-api, alongside PointNav. This PR also serves as a reference point on how to adding new tasks in the future.
Huge thanks to Medhini Narasimhan @medhini for formalizing RoomNav task, creating dataset and making this PR possible! A tiny RoomNav dataset is so available here, courtesy of Medhini 🎉🎉
What Were Added?
To support a new task like room navigation, the following has been added:
habitat/task/.RoomNavRLEnvadded inhabitat_baselines/common/environments.py.habitat/datasets/roomnav/roomnav_dataset.pyand roomnav.zip.habitat/configs/tasks/,habitat.configs/default.py,habitat_baselines/configs/roomnav/andhabitat_baselines/configs/default.py.TODOs:
How Has This Been Tested
Pytest roomnav train/tests runs pass.
Types of changes
Checklist